fix(gitlab): pin commit statuses to the same pipeline via pipeline_id caching#2671
fix(gitlab): pin commit statuses to the same pipeline via pipeline_id caching#2671ab-ghosh wants to merge 1 commit intotektoncd:mainfrom
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2671 +/- ##
==========================================
+ Coverage 58.85% 58.91% +0.05%
==========================================
Files 204 204
Lines 20149 20177 +28
==========================================
+ Hits 11859 11887 +28
Misses 7525 7525
Partials 765 765 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a caching mechanism for GitLab pipeline IDs to ensure that multiple commit statuses for the same project and SHA are grouped within the same pipeline. It includes logic to retrieve, store, and apply these IDs during the status creation process, along with comprehensive unit tests. A critical issue was identified in the setOptPipelineID function where a stale pipeline ID from a source project could persist when falling back to a target project, potentially causing API failures.
ab-ghosh
force-pushed
the
fix/gitlab-commit-status-pipeline-id-caching
branch
from
071640b to
f57af59
Compare
🤖 AI Analysis - pr-complexity-ratingBased on the provided information, this PR appears to be a synchronization/maintenance operation rather than a functional feature or bug fix implementation. 📊 PR Review Complexity
Overall difficulty: Easy SummaryThis PR is a merge commit synchronizing Suggested reviewers focus
Generated by Pipelines-as-Code LLM Analysis |
🤖 AI Analysis - pr-complexity-ratingBased on the provided context, this PR appears to be a maintenance merge (synchronizing the feature branch with 📊 PR Review Complexity
Overall difficulty: Easy SummaryThis PR consists of a merge commit synchronizing a feature branch ( Suggested reviewers focus
Generated by Pipelines-as-Code LLM Analysis |
|
I am not sure I understand this PR, can you explain it to me? 😅 Your change get pipeline_id only remembered inside the current in-memory GitLab provider object. That helps only while that exact object is still being used. In PAC, the later/final status update is sent from a different phase of the system that creates a non shared new GitLab Provider{} instance, so that cached pipeline_id is lost. In practice, this means the change may help for a few status calls in the same immediate flow, but it likely won’t keep all statuses for the PipelineRun attached to the same GitLab pipeline, which I think your goal. Maybe we need to carry the pipeline_id via an annotation and getting read by the reconciler? |
The issue which this PR addresses that when PAC posts commit statuses to GitLab for a merge request, the status updates can get split across two different GitLab pipelines. At some point during the continuous stream of status updates, GitLab stops associating them with the original MR pipeline and starts routing them to a new external pipeline. The original MR pipeline gets permanently stuck with stale intermediate statuses, while subsequent results sit in an orphaned pipeline invisible to the MR. |
yeah that's right, i missed this point, i think, as you suggested, after the first successful status update, we can add the pipeline_id via an annotation and then any subsequent CreateStatus call can reads it. |
When PAC posts multiple commit statuses for the same SHA, GitLab's auto-assignment logic can route them to different pipelines, leaving the MR pipeline permanently stuck with stale intermediate statuses. Cache the pipeline_id returned by the first SetCommitStatus response and pass it on subsequent calls for the same (project, SHA) pair so all statuses land in the same GitLab pipeline. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Abhishek Ghosh <abghosh@redhat.com>
ab-ghosh
force-pushed
the
fix/gitlab-commit-status-pipeline-id-caching
branch
from
faba1a9 to
a36c209
Compare
|
can you make sure this is tested manually properly? without having to rely on the AI words? |
|
@ab-ghosh is your goal to have pipeline_id only for an event or for whole CI lifecycle of the merge request? |
4 participants
📝 Description of the Change
When PAC posts commit statuses to GitLab for a merge request, the status updates can get split across two different GitLab pipelines. GitLab's auto-assignment logic can abruptly change routing mid-stream, leaving the original MR pipeline permanently stuck with stale intermediate statuses.
This fix caches the
pipeline_idreturned by the firstSetCommitStatusresponse for a given(project, SHA)pair and passes it on all subsequent calls, ensuring all commit statuses land in the same GitLab pipeline.🔗 Linked GitHub Issue
Fixes #
https://redhat.atlassian.net/browse/SRVKP-11437
🧪 Testing Strategy
🤖 AI Assistance
AI assistance can be used for various tasks, such as code generation,
documentation, or testing.
Please indicate whether you have used AI assistance
for this PR and provide details if applicable.
Important
Slop will be simply rejected, if you are using AI assistance you need to make sure you
understand the code generated and that it meets the project's standards. you
need at least know how to run the code and deploy it (if needed). See
startpaac to make it easy
to deploy and test your code changes.
If the majority of the code in this PR was generated by an AI, please add a
Co-authored-bytrailer to your commit message.For example:
Co-authored-by: Claude noreply@anthropic.com
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.